Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for testing emails sent from Drupal #392

Merged
merged 13 commits into from
Mar 19, 2018

Conversation

jonathanjfshaw
Copy link
Contributor

Addresses #195.
Depends on parallel driver PR134: jhedstrom/DrupalDriver#134.

A few architectural notes ...

Service architecture

I had in mind the possibility that there could be fundamentally different mechanisms for handling test emails (e.g. Drupal core's test mail collector, contrib maillog, and IMAP) which could use the same context. Therefore I tried to put interacting with the mail store outside of the context as a service. The supplied implementation uses test mail collector.

However, I'm not clear yet on how do this in the Drupal extension and so it's not a real service for now, just a kind of service-like class. Hopefully though it will be relatively easy to upgrade to a real service in the future.

Abstracting in this way is responsible for a few minor complications that have no purpose in the default implementation such as

  • the distinction between enabling/disabling email sending, and collecting/not-collecting emails. An IMAP service would want to send emails AND collect them, whereas for Drupal's test mail collector collecting always stops sending.
  • the $store name passed when getting emails. Test mail collector is just one store, but different services, or combinations of services, could potentially be capturing emails into various different stores.

Mink context

I needed to be able to visit pages in order to follow links from emails, but the context is extending RawDrupalContext not RawMinkContext. Therefore I moved getcontext() out of SubContextBase into it's parent RawDrupalContext and used this to help me load a minkContext and visit a page. Maybe there was an easier way of doing this.

@jonathanjfshaw
Copy link
Contributor Author

There's a lot of helper functions in the MailContext. It's possible that these should be split out into a RawMailContext in order to make it easier for people to override the mail steps in their own projects.

@jhedstrom
Copy link
Owner

This is looking great! Thanks for working on this.

I agree that any non-step-definition code in MailContext should go into a RawMailContext to allow that to be re-used for folks that want to craft their own step-definitions.

@pfrenssen
Copy link
Collaborator

I needed to be able to visit pages in order to follow links from emails, but the context is extending RawDrupalContext not RawMinkContext. Therefore I moved getcontext() out of SubContextBase into it's parent RawDrupalContext and used this to help me load a minkContext and visit a page. Maybe there was an easier way of doing this.

This is also a good idea! Can we maybe split this off to a separate PR?

@jonathanjfshaw
Copy link
Contributor Author

Suggested changes by @jhestrom and @pfrenssen made. Tests are failing because this is blocked on the hunk split out into #396 and the supporting driver methods in jhedstrom/DrupalDriver#134.

@jhedstrom
Copy link
Owner

Thanks for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants